fix(mcp-workforce): align memory scope enum with narrowed PersonaMemoryScope#105
Closed
khaliqgant wants to merge 1 commit into
Closed
fix(mcp-workforce): align memory scope enum with narrowed PersonaMemoryScope#105khaliqgant wants to merge 1 commit into
khaliqgant wants to merge 1 commit into
Conversation
mcp-workforce was landed in #91 against an older `PersonaMemoryScope` shape (`session | user | workspace | org | object`). #94 then tightened the type to `workspace | user | global`. Both PRs passed CI independently, but main is now broken at build time because the zod enum in `server.ts` and the runtime `VALID_SCOPES` Set in `tools/memory.ts` still reference the removed literals. Aligning both call sites to the canonical persona-kit shape: - `MEMORY_SCOPE_ENUM` → z.enum(['workspace', 'user', 'global']) - `VALID_SCOPES` → new Set(['workspace', 'user', 'global']) - memory.save tool description updated to match The default scope stays `workspace`. Callers that previously passed `'session'`/`'org'`/`'object'` will now get a validation error from the zod schema before the runtime check — preferable to silently mapping them to a different scope. Verified: `pnpm -F @agentworkforce/mcp-workforce typecheck` + `build` + `test` (23/23) all pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR narrows the ChangesMemory Scope Narrowing
🎯 1 (Trivial) | ⏱️ ~3 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/mcp-workforce/src/server.ts (1)
13-13: ⚡ Quick winSingle-source the memory scope literals to prevent future drift.
This PR fixes a drift issue, but the literals are still duplicated between
server.tsandtools/memory.ts. Please centralize them in one exportedas consttuple and derive both the zod enum and runtime validation from it.♻️ Suggested refactor
-const MEMORY_SCOPE_ENUM = z.enum(['workspace', 'user', 'global']); +import { MEMORY_SCOPES } from './tools/memory.js'; +const MEMORY_SCOPE_ENUM = z.enum(MEMORY_SCOPES);+export const MEMORY_SCOPES = ['workspace', 'user', 'global'] as const; const VALID_SCOPES: ReadonlySet<PersonaMemoryScope> = new Set([ - 'workspace', - 'user', - 'global' + ...MEMORY_SCOPES ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mcp-workforce/src/server.ts` at line 13, The MEMORY_SCOPE_ENUM literal array is duplicated; centralize the allowed scope strings by exporting a single as const tuple (e.g. MEMORY_SCOPES) from tools/memory.ts, then replace the local z.enum([...]) in server.ts with z.enum(MEMORY_SCOPES) (or z.enum([...MEMORY_SCOPES]) if needed) and derive any runtime type/validation from that same exported tuple (update imports and usages of MEMORY_SCOPE_ENUM to use MEMORY_SCOPES and its derived types), ensuring all places that previously referenced MEMORY_SCOPE_ENUM now import the single source of truth from tools/memory.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/mcp-workforce/src/server.ts`:
- Line 13: The MEMORY_SCOPE_ENUM literal array is duplicated; centralize the
allowed scope strings by exporting a single as const tuple (e.g. MEMORY_SCOPES)
from tools/memory.ts, then replace the local z.enum([...]) in server.ts with
z.enum(MEMORY_SCOPES) (or z.enum([...MEMORY_SCOPES]) if needed) and derive any
runtime type/validation from that same exported tuple (update imports and usages
of MEMORY_SCOPE_ENUM to use MEMORY_SCOPES and its derived types), ensuring all
places that previously referenced MEMORY_SCOPE_ENUM now import the single source
of truth from tools/memory.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 922fa5f2-6768-43e7-a49e-c74000d37e08
📒 Files selected for processing (2)
packages/mcp-workforce/src/server.tspackages/mcp-workforce/src/tools/memory.ts
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Member
Author
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why CI is red on main
mcp-workforce(added in #91) was authored against an olderPersonaMemoryScopeshape —'session' | 'user' | 'workspace' | 'org' | 'object'.#94then narrowed the type to'workspace' | 'user' | 'global'. Each PR passed CI independently. After both merged, main fails to build:This is the exact case CodeRabbit flagged on #94 (
packages/persona-kit/src/types.ts:186— "remaining uses of removed literals"). At the time of the #94 sweep,mcp-workforcewasn't yet on main, so the audit returned clean. The cross-PR collision only surfaces post-merge.What this changes
Two call sites in
mcp-workforce, both pointing at the canonical persona-kit shape:src/server.ts:13z.enum(['session', 'user', 'workspace', 'org', 'object'])z.enum(['workspace', 'user', 'global'])src/server.ts:65src/tools/memory.ts:38-44Setof 5 old literalsSetof 3 canonical literalsmemory.save's default scope staysworkspace.Behavior changes for consumers
Callers that previously passed
'session','org', or'object'will now get azodvalidation error before the runtime check — preferable to silently mapping to a different scope. No mapping layer is added because none of these old values were in production (mcp-workforce is0.0.0on npm — a placeholder).Verified locally
pnpm -F @agentworkforce/mcp-workforce typecheck— cleanpnpm -F @agentworkforce/mcp-workforce build— cleanpnpm -F @agentworkforce/mcp-workforce test— 23/23 pass🤖 Generated with Claude Code